Repository navigation
Preserve first timestamp after fallback - #593
Harsh (Mr-KnightCLI) wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
A JUnit regression test is still needed to verify that an early getTimeOfFirstEvent() call returns zero without preventing the getter and diary from recording and retaining the first real timestamp.
1 open issue
What changed in this PR
This PR updates GCToolKit’s pre-unified log diarizer to preserve timestamp discovery after an early lookup, addressing the cached-fallback behavior described in #196.
Changes:
- Returns zero as a temporary fallback without storing it as the first-event timestamp.
- Adds a comment explaining why later log lines must remain able to set the timestamp.
| File | Description |
|---|---|
parser/src/main/java/com/microsoft/gctoolkit/parser/jvm/PreUnifiedDiarizer.java |
Makes the zero fallback transient so a later parsed timestamp can be recorded. |
Review effort: Balanced
|
Harsh (@Mr-KnightCLI) are you able to add a unit test as suggested? |
|
Harsh (@Mr-KnightCLI) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
|
Thanks, Martijn Verburg (@karianna). I added the JUnit regression test in 9fe8113. It checks that an early |
|




Fixes #196.
getTimeOfFirstEvent()may be called before a timestamped line is parsed. The zero fallback was being cached intimeOfFirstEvent, which prevented a real timestamp found later in the log from being recorded.Return the zero timestamp as a transient fallback so later log lines can still set the actual first-event timestamp.
Validation:
git diff --checkpassed. Tests were not run.